-
Couldn't load subscription status.
- Fork 21
[TRAIN-3456] Add healthchecks. Add functionality to Makefile. #122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…tation; add transform_compose.py script and unit tests for compose transformations.
…ddprofrb for profiling
…nction for generated files in transform_compose.py
…put setting in next.js for prod build
…production frontend build
…rget, and build args in docker-compose template; refine regex pattern for build section replacement in transformation logic.
…opment and production environments, including environment variable configurations and dependency installations.
… function for selecting the appropriate Docker Compose file based on the ENV variable. Simplify commands for starting, stopping, and managing containers in both development and production environments.
…ariables for frontend service; add port mapping for local development.
…th NEXT_PUBLIC_DD variables for frontend service configuration.
…ment to production
…nv var stuff to env templates, move feature stuff to feature.md
…BLIC_DD variables for consistency in frontend service configuration.
… with NEXT_PUBLIC_DD variables for consistency in frontend service configuration.
| console.error(e) | ||
| setLoading(false) | ||
| } | ||
| }, [adsPath, getRandomArbitrary, setData, setLoading]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React said that this wasn't needed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, it's not a dependency
| @@ -1,29 +1,134 @@ | |||
| FROM node:20 AS builder | |||
| FROM node:lts-alpine AS base | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To mitigate a security vulnerability in other image
| @@ -1,4 +1,5 @@ | |||
| module.exports = { | |||
| output: 'standalone', | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for the optimized production builds (production build target)
… NEXT_PUBLIC prefix and enhance transformation logic for development to production changes.
|
|
||
| settings.json | ||
|
|
||
| docker-compose.dev.frontend-prod.yml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this last one left here on purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is only for testing the frontend prod build target.
Can't test it with the regular docker-compose.yml because it's not in the storedog image.
Can't test it with the docker-compose.dev.yml because of the volume mounts.
I'm adding a script and a make command to generate this file, but there's no reason to add it to source control.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For more context, the prod build creates a server.js file and the command is node server.js. If we're mounting the entire service directory, we end up losing that file.
| labels: | ||
| com.datadoghq.ad.logs: '[{"source": "python"}]' | ||
| com.datadoghq.ad.logs: '[{"source": "python", "service": "store-discounts"}]' | ||
| com.datadoghq.tags.env: '${DD_ENV:-development}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we're adding service level com.datadoghq.tags.* back in some definitions and not others? IIRC we removed them not long ago because Shawn said they were redundant if we had proper labelling elsewhere (e.g. setting DD_ENV at the agent-level means all other services will follow suit).
I could be mistaken on this, and keeping them doesn't do any harm, so not really a blocker imo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was an accident on my part. I was using my RUM course's branch as a starting point, but there have been updates since that branch was made.
|
|
||
|
|
||
| .gradle/ | ||
| Gemfile.lock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You remove Gemfile.lock but since it's already been checked-in, it won't retroactively ignore it. Need to either delete the file (this is fine, because the docker image sets it up anyway) or reset the git cache so the .gitignore gets adhered to.
Also, I think to avoid this in the future, we should probably create/update a .dockerignore file that has node_modules, .git, and Gemfile.lock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol oh yeah duh 🤦 ... I don't know enough about Gemfile.lock. Is that something we want under source control?
…g, and improve documentation for better usability.
…og configurations for cleaner logging setup.
…, simplifying the setup for Datadog logging.
…et to simplify usage instructions.
Description
docker-compose.ymlfiles fromdocker-compose.dev.yaml. (One is intended for pre-release, another is for testing the frontend prod build target while using the dev docker compose file.)env.development.templatefor TCD use.Frontend build times
Tested with the help of Cursor/Claude.
Image Sizes
Tested with the help of Cursor/Claude.
How to test
Test the healthchecks
make upto build the dev containers.Test the frontend prod build target
Makefile, setENV=dev-frontend-prod.make up clean.Test the dockerfile generation script
make prepare-release.docker-compose.generated.ymlfile and answer Y/N in the terminal.Test the Makefile updates
make helpto see the commands.